Skip to content

First ECAL implementation#120

Closed
kharlov wants to merge 6 commits into
AliceO2Group:masterfrom
kharlov:master
Closed

First ECAL implementation#120
kharlov wants to merge 6 commits into
AliceO2Group:masterfrom
kharlov:master

Conversation

@kharlov

@kharlov kharlov commented Sep 11, 2021

Copy link
Copy Markdown

No description provided.

@njacazio

Copy link
Copy Markdown
Collaborator

Hi @kharlov please trigger me when you need to have this PR reviewed

@kharlov

kharlov commented Sep 23, 2021

Copy link
Copy Markdown
Author

Hi @njacazio , I fixed several bugs in ECAL response, please check and merge the new PR.

@njacazio njacazio left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kharlov good work! Could you also run clang-format -i on the files that you changed so as to uniformize the style?
If it does not work it should be possible to do it from inside the alienv environment

Comment thread examples/aod/createO2tables.C Outdated
Comment on lines +42 to +47
const double Bz = 5.e-1;
// TOF
const double tof_radius = 100.; // [cm] Radius of the TOF detector (used to compute acceptance)
const double tof_length = 200.; // [cm] Length of the TOF detector (used to compute acceptance)
const double tof_sigmat = 0.02; // [ns] Resolution of the TOF detector
const double tof_sigmat0 = 0.2; // [ns] Time spread of the vertex
const double tof_radius = 100.;
const double tof_length = 200.;
const double tof_sigmat = 0.020;
const double tof_sigmat0 = 0.20;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not need to be changed

// Simulation parameters
constexpr bool do_vertexing = true; // Vertexing with the O2
constexpr bool enable_nuclei = true; // Nuclei LUTs
constexpr bool enable_nuclei = true;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not need to be changed

// info for the ECAL
float posZ, posPhi;
TLorentzVector pECAL;
if (ecal_detector.makeSignal(*particle, pECAL, posZ, posPhi)) { // to be updated 13.09.2021

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initalization of the TLorentzVector could be done once and used outside of the loop for tracks

@njacazio

Copy link
Copy Markdown
Collaborator

Also, there are some conflicts that have to solved before this can be merged

@kharlov

kharlov commented Sep 23, 2021

Copy link
Copy Markdown
Author

Hi @njacazio , I solved conflicts, ran clang and opened a new PR. Did I do it correctly?

@njacazio

Copy link
Copy Markdown
Collaborator

Hi Yuri, yes, I will have a look asap

@njacazio

Copy link
Copy Markdown
Collaborator

Hi @kharlov your changes will be merged from here #126

@kharlov

kharlov commented Sep 27, 2021

Copy link
Copy Markdown
Author

Hi @njacazio , I fixed one more bug in ECAL response: photon hit coordinate was not calculated correctly.

@njacazio

Copy link
Copy Markdown
Collaborator

Hi, @kharlov, perfect, I will include it in the software as soon as you push it in this PR

@njacazio

Copy link
Copy Markdown
Collaborator

Ah, I just saw that you already did, I will import it now

@njacazio

Copy link
Copy Markdown
Collaborator

Closing as already merged in other PR

@njacazio njacazio closed this Sep 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants